Absorb elements from the validated input slice - #41
Merged
Merged
Conversation
There was a problem hiding this comment.
🟢 Approval recommended
No unresolved review issues remain.
Pull request overview
Fixes Sponge::absorb by reusing the slice validated for length, preventing stateful AsRef inputs from being inconsistently handled.
Changes:
- Reuse the validated input slice.
- Add a regression test for stateful
AsRefinputs. - Document the fix in the changelog.
File summaries
| File | Description |
|---|---|
tests/sponge.rs |
Adds regression coverage. |
src/sponge.rs |
Reuses the validated input slice. |
CHANGELOG.md |
Records the fix. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
moCello
approved these changes
Sep 14, 2026
moCello
left a comment
moCello
left a comment
Member
There was a problem hiding this comment.
APPROVE — The fix is complete: absorb now checks and absorbs one borrowed slice. The author reports the regression failing on unmodified main in debug and release, expecting 9 and getting 0, and passing with the fix.
Nothing gates the merge. Two small notes follow, one on a test import and one on the changelog wording. Neither affects behavior.
Read AsRef input once so validation and absorption use the same view. A changing-view adapter could previously satisfy the length check and then supply no elements while absorb still reported success. Add a regression using the existing Rotate backend and record the fix in the changelog.
HDauven
force-pushed
the
fix/absorb-validated-input
branch
from
September 14, 2026 13:46
4fdc3c5 to
079b5f9
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Borrow the input once in
Sponge::absorb, so the slice checked for sufficient length is also the slice that is absorbed.Previously, the two
as_ref()calls could return different slices. A safe Rust adapter that returns two elements on its first call and an empty slice on the second makesabsorb(2, ...)report success without absorbing either element.This behavior dates to SAFE's initial implementation; it was not introduced by #40. This PR is based directly on current
main, after #40.Changes
No new API, dependencies, allocations or changes to field/permutation logic. Ordinary slice/array inputs retain their existing behavior. The diff is three files, +23/-2 lines.
Validation
mainin debug and release (expected 9, actual 0), and passes with the fix.make test,make cq, no-default-features tests, no_std builds with and without encryption, docs and frozen-vector checks pass.Investigation harnesses, fuzz corpora and mutation tests are intentionally outside this PR. These are bounded correctness checks, not a proof of cryptographic security; no deployed impact or authentication bypass was demonstrated.